Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix a PHP Notice in REST API responses #6576

Merged
merged 2 commits into from
May 4, 2018
Merged

Fix a PHP Notice in REST API responses #6576

merged 2 commits into from
May 4, 2018

Conversation

pento
Copy link
Member

@pento pento commented May 4, 2018

Description

As described in #6575, there's a PHP notice when adding the block_format to post responses.

We can fix this by removing the is_array() check, is isset() can check multi-dimensional arrays correctly.

@pento pento added [Type] Bug An existing feature does not function as intended Core REST API Task Task for Core REST API efforts labels May 4, 2018
@pento pento added this to the 2.9 milestone May 4, 2018
@pento pento self-assigned this May 4, 2018
@pento pento requested a review from danielbachhuber May 4, 2018 06:53
@gziolo
Copy link
Member

gziolo commented May 4, 2018

  1. REST_Blocks_Controller_Test::test_create_item
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -'

    Wow!

    '
    +'0!-- wp:core/paragraph -->

    Wow!

    '
    /home/travis/build/WordPress/gutenberg/phpunit/class-rest-blocks-controller-test.php:136
    /home/travis/.phpbrew/php/php-5.2.17/lib/php/phpunit/phpunit.php:46
  2. REST_Blocks_Controller_Test::test_update_item
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -'

    Nice!

    '
    +'0!-- wp:core/paragraph -->

    Nice!

    '
    /home/travis/build/WordPress/gutenberg/phpunit/class-rest-blocks-controller-test.php:165
    /home/travis/.phpbrew/php/php-5.2.17/lib/php/phpunit/phpunit.php:46
  3. REST_Blocks_Controller_Test::test_delete_item
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -'

    Hello!

    '
    +'0!-- wp:core/paragraph -->

    Hello!

    '
    /home/travis/build/WordPress/gutenberg/phpunit/class-rest-blocks-controller-test.php:193
    /home/travis/.phpbrew/php/php-5.2.17/lib/php/phpunit/phpunit.php:46
    FAILURES!
    Tests: 175, Assertions: 287, Failures: 3, Skipped: 9.

PHPUnit doesn't like it :)

@ocean90
Copy link
Member

ocean90 commented May 4, 2018

The is_array() was added for PHP 5.2 compat, see 75e7234.

@gziolo
Copy link
Member

gziolo commented May 4, 2018

So you need another isset in that case in additon to is_array check ...

@gziolo
Copy link
Member

gziolo commented May 4, 2018

It fixes the issue.

@gziolo gziolo merged commit dd7dabd into master May 4, 2018
@gziolo gziolo deleted the fix/6575-php-warning branch May 4, 2018 10:17
@gziolo gziolo mentioned this pull request May 4, 2018
Closed
@pento
Copy link
Member Author

pento commented May 4, 2018

PHP 5.2 is the worst.

@ZebulanStanphill
Copy link
Member

Off-topic, but why does WordPress still support such old PHP versions? PHP 5.5 and earlier is not even officially supported anymore. Couldn't the dev team announce that PHP 5.5 and earlier versions will stop being supported in WordPress 5.2 or something, in order to give people time to update? I've heard some people complain that WordPress is actually holding back adoption of newer PHP versions by continuing to support really old PHP versions.

@danielbachhuber
Copy link
Member

Hi @SuperGeniusZeb,

Let's keep the conversation on-topic. For discussion of minimum PHP version, please this existing thread https://core.trac.wordpress.org/ticket/33381

This was referenced May 11, 2018
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielbachhuber danielbachhuber Awaiting requested review from danielbachhuber

Assignees

@pento pento

Labels
Core REST API Task Task for Core REST API efforts [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
2.9
Development

Successfully merging this pull request may close these issues.
5 participants
@pento @gziolo @ocean90 @ZebulanStanphill @danielbachhuber